Skip to content

feat(local): add --verify, --timeout, auto-detect dev script, post-init verification#998

Merged
MathurAditya724 merged 43 commits into
mainfrom
feat/local-run-verify
Jun 5, 2026
Merged

feat(local): add --verify, --timeout, auto-detect dev script, post-init verification#998
MathurAditya724 merged 43 commits into
mainfrom
feat/local-run-verify

Conversation

@MathurAditya724

Copy link
Copy Markdown
Member

Summary

Adds dev script auto-detection and post-init verification to sentry local run:

  • Auto-detect: when no command is provided, detects the dev script from package.json (dev > develop > serve > start), manage.py, app.py/main.py, go.mod, or docker-compose.yml
  • --verify flag: starts a local server, runs the app, waits for the first SDK envelope to arrive, then reports success/failure
  • --timeout flag: kills the child process after N seconds
  • Post-init verification: after sentry init succeeds, automatically runs the detected dev command with --verify --timeout 30 to confirm the SDK is sending events. On failure, captures a Sentry event with diagnostic context.

Testing

  • 21 tests pass across 3 files (dev-script unit + property, run command)
  • bun run typecheck, check:deps, check:errors, check:fragments all pass

…it verification

- Add src/lib/dev-script.ts: auto-detects dev command from package.json
  (dev > develop > serve > start), manage.py, app.py, main.py, go.mod,
  docker-compose.yml/compose.yml
- Update sentry local run: when no command args provided, auto-detect
  from the project. Add --verify flag (wait for first SDK event, then
  exit) and --timeout flag (kill child after N seconds)
- Add src/lib/init/verify-setup.ts: after successful sentry init, run
  the detected dev command with --verify to confirm SDK sends events.
  On failure, capture a Sentry event with diagnostic context.
- Wire verify-setup into wizard-runner.ts handleFinalResult()
- 21 tests across 3 files (dev-script unit + property, run command)
@github-actions

github-actions Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://cli.sentry.dev/_preview/pr-998/

Built to branch gh-pages at 2026-06-05 01:11 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@MathurAditya724

Copy link
Copy Markdown
Member Author

fix-ci: attempt 1 — tests use /tmp/opencode for temp dirs which doesn't exist in CI. Switching to TEST_TMP_DIR from test/constants.ts (uses os.tmpdir()).

Tests were using /tmp/opencode which only exists in the local dev
environment. CI runners don't have this directory, causing mkdtemp
to fail. Switch to TEST_TMP_DIR from test/constants.ts which uses
os.tmpdir() and is worker-scoped for parallel test isolation.
@github-actions

github-actions Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

Codecov Results 📊

❌ Patch coverage is 46.38%. Project has 4758 uncovered lines.
❌ Project coverage is 81.58%. Comparing base (base) to head (head).

Files with missing lines (4)
File Patch % Lines
src/lib/init/verify-setup.ts 8.11% ⚠️ 136 Missing and 1 partials
src/commands/local/run.ts 80.00% ⚠️ 22 Missing and 7 partials
src/lib/dev-script.ts 89.74% ⚠️ 4 Missing and 1 partials
src/lib/init/wizard-runner.ts 85.71% ⚠️ 1 Missing and 2 partials
Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
- Coverage    82.00%    81.58%    -0.42%
==========================================
  Files          367       369        +2
  Lines        25537     25829      +292
  Branches     16750     16884      +134
==========================================
+ Hits         20939     21071      +132
- Misses        4598      4758      +160
- Partials      1760      1770       +10

Generated by Codecov Action

Comment thread src/commands/local/run.ts
Comment thread src/commands/local/run.ts
Comment thread src/commands/local/run.ts
…p scripts with env vars

- Store and clearTimeout after Promise.race resolves in runWithVerify
  and verifySetup to prevent holding the event loop alive
- Add SIGINT/SIGTERM forwarding in runWithVerify so Ctrl-C kills the
  child instead of orphaning it
- Detect shell features (env-var prefixes, pipes, operators) in
  package.json scripts and run them via sh -c instead of naive
  whitespace splitting
Comment thread src/lib/init/verify-setup.ts Outdated
Comment thread src/lib/init/wizard-runner.ts
- Register SIGINT/SIGTERM handlers in verifySetup so Ctrl-C during
  post-init verification kills the child instead of orphaning it
- Await child.exited after SIGTERM in both verifySetup and
  runWithVerify (envelope/timeout branches) so the child releases
  its port before the function returns
Comment thread src/commands/local/run.ts Outdated
Comment thread src/lib/dev-script.ts
- Always add a timeout racer in runWithVerify — defaults to 30s when
  no explicit --timeout is given, preventing indefinite hangs
- Redact KEY=VALUE env-var assignments in the detectedCommand
  telemetry field to avoid leaking secrets from package.json scripts
Comment thread src/commands/local/run.ts Outdated
Use named handler references and removeListener after the race
settles so SIGINT/SIGTERM aren't swallowed during teardown.
Comment thread src/lib/init/verify-setup.ts Outdated
Prevents indefinite hangs when the dev server doesn't respond to
SIGTERM. Extracted gracefulKill helper in run.ts; inlined the same
pattern in verify-setup.ts.
@MathurAditya724

Copy link
Copy Markdown
Member Author

fix-ci: attempt 1 — two lint errors (numeric separators 5_0005000) and a property test failure in sentryclirc-import.property.test.ts with counterexample "*", investigating

- Replace 5_000 with 5000 to satisfy biome useNumericSeparators rule
- Skip all-asterisk inputs in maskToken identity test (masking '*' correctly returns '*')
@MathurAditya724

Copy link
Copy Markdown
Member Author

pushed fix in 02b8918 — removed 5_000 numeric separators (biome lint) and narrowed the maskToken property test to skip all-asterisk inputs (masking "*" correctly returns "*", so the identity assertion was too broad).

Comment thread src/commands/local/run.ts Outdated
Store and clearTimeout the SIGKILL grace timer so it doesn't hold
the event loop alive for 5s after a cooperative child exit.
@MathurAditya724

Copy link
Copy Markdown
Member Author

fix-ci: attempt 3 — biome flagged 5_000 as an unnecessary numeric separator in verify-setup.ts:136. Changed to 5000.

Comment thread src/commands/local/run.ts
Comment thread src/commands/local/run.ts Outdated
Comment thread src/lib/init/verify-setup.ts Outdated
…n env redaction

- Add $ and lowercase letters to SHELL_FEATURES_RE so scripts with
  variable references or mixed-case env assignments route through sh -c
- Wrap gracefulKill's initial SIGTERM in try/catch so an already-exited
  child doesn't skip shutdownServer
- Broaden telemetry redaction regex to match mixed-case env var names
Comment thread src/commands/local/run.ts
Comment thread src/commands/local/run.ts Outdated
github-actions Bot and others added 2 commits May 21, 2026 16:13
Move removeListener calls after the child kill/await so SIGINT
during the 5s grace period still forwards to the child.
Comment thread src/commands/local/run.ts Outdated
Comment thread src/commands/local/run.ts
- Clear the watchChildOutput timer when settle() is called from any
  path (fatal error, close event), preventing a 15s stall on exit.

- Broaden ENV_VAR_RE to KEY_VALUE_RE to also catch --flag=value CLI
  arguments (e.g. --api-key=secret) in telemetry redaction.

- Fix URI_USERINFO_RE to handle empty-username URIs like
  redis://:password@host by allowing zero chars before the colon.

- Reuse scrubOutputLine() for detectedCommand telemetry field instead
  of a separate inline regex.
Comment thread src/commands/local/run.ts Outdated
Comment thread src/lib/init/verify-setup.ts Outdated
- Add exitCode check before awaiting close after SIGKILL in
  gracefulKill() to prevent indefinite hang if the close event
  fires between the kill call and listener attachment.

- Scrub the error line shown to the user via ui.log.warn with
  scrubOutputLine() to redact credentials that may appear in
  startup errors (e.g. database connection strings in CI logs).
Comment thread src/lib/init/verify-setup.ts
Comment thread src/lib/init/wizard-runner.ts
Comment thread src/lib/init/wizard-runner.ts
- cleanupChild now awaits the close event after SIGKILL so
  child.exitCode is populated before the crash detection check
  in verifySetup. Without this, a force-killed process would have
  exitCode=null, bypassing the started→exited correction.

- Fix biome formatting (single-line if condition).
- Read child.exitCode before cleanupChild() so the crash detection
  sees the natural exit code, not 143/137 from our SIGTERM/SIGKILL.
  Fixes false failures where a healthy server killed by cleanup was
  reported as crashed.

- Wrap verifySetup() call in wizard-runner with try-catch so
  unexpected throws from third-party calls (createSpotlightBuffer,
  buildApp) don't leave the spinner running or skip formatResult.
Comment thread src/lib/dev-script.ts
- Trim package.json script value before testing against
  SHELL_FEATURES_RE so leading whitespace doesn't prevent detection
  of env-var assignments (e.g. '  NODE_OPTIONS=... tsx dev').

- Remove unused biome-ignore suppression for useMaxParams in
  verify-setup.ts (reportOutcome now takes 2 params via ctx object).

- Remove unused biome-ignore suppression for noControlCharactersInRegex
  in local.ts (biome no longer flags this regex).

- Fix import sort order in wizard-runner.ts after adding logger.

@sentry-warden sentry-warden Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unhandled child process error event in verifySetup causes process crash

In src/lib/init/verify-setup.ts, the spawned child process has no error event listener; an async spawn error (ENOENT, EACCES, etc.) will become an uncaught exception and crash the CLI. Add a child.on('error', ...) handler before entering the Promise.race.

Evidence
  • verify-setup.ts attaches child.on('close') via childExited (and indirectly via watchChildOutput), but grep finds zero child.on("error") registrations in the file.
  • watchChildOutput only registers child.stdout/stderr data and child.on('close') listeners, leaving the error event unhandled.
  • On Linux, a missing executable does not always throw synchronously from spawn(); the error surface is equivalent to the same bug in run.ts:runWithVerify.
  • Node.js EventEmitter promotes an unhandled error event to an uncaught exception.

Identified by Warden find-bugs

Comment thread src/commands/local/run.ts
Comment thread src/lib/dev-script.ts Outdated
Change ^[A-Za-z_]+= to ^[A-Za-z_]\w*= so scripts like
E2E_BASE_URL=... or API_V2_KEY=... are correctly detected as
needing shell execution instead of being whitespace-split.
Comment thread src/lib/dev-script.ts
Comment thread src/lib/init/verify-setup.ts
Extend the post-race exit code correction to also cover the 'silent'
outcome. A child that crashes immediately with no output would resolve
as 'silent' (from watchChildOutput's close handler) instead of
'exited'. Now both 'started' and 'silent' are corrected to 'exited'
when preCleanupExitCode is non-zero.
Comment thread src/lib/dev-script.ts
BYK added a commit that referenced this pull request Jun 4, 2026
…ss capture and -f ai filter (#1034)

## Summary

`sentry local` works well, but a few behaviors were undocumented and
tripped up a user debugging logs/traces locally. This started as a docs
fix and now also closes a real gap in client-side wiring.

- **No DSN required.** With no DSN configured, events flow **only** to
the local server — nothing reaches the user's Sentry org and no
production quota is used. With a DSN set, the SDK sends to both. This is
the main reassurance for using spotlight in local dev and wasn't stated.
- **Inject every framework client prefix (behavior change).** `sentry
local run` previously injected only `NEXT_PUBLIC_SENTRY_SPOTLIGHT`. It
now injects the spotlight URL under **all** common framework client
prefixes so the URL is inlined into the browser bundle regardless of
bundler:
  - `PUBLIC_` (SvelteKit, Astro, Qwik)
  - `NEXT_PUBLIC_` (Next.js)
  - `VITE_` (Vite)
  - `NUXT_PUBLIC_` (Nuxt)
  - `REACT_APP_` (Create React App)
  - `VUE_APP_` (Vue CLI)
  - `GATSBY_` (Gatsby)

This mirrors the prefix set from
[getsentry/sentry-javascript#18198](getsentry/sentry-javascript#18198).
`SENTRY_SPOTLIGHT` (read by server-side SDKs) is still set and is
applied last so the base name is never shadowed by a client variant.
- **Client-side wiring caveat, reframed.** No `@sentry/*` package reads
these client vars *yet* — only `@sentry/node-core` reads
`SENTRY_SPOTLIGHT` (server-side). The previous docs framed manual wiring
as a permanent requirement; it's actually the **current workaround until
the browser SDK reads these vars automatically** (#18198, which this PR
is a good opportunity to revive). Until then, reference the variable for
your framework:
  ```ts
Sentry.init({ spotlight: process.env.NEXT_PUBLIC_SENTRY_SPOTLIGHT ??
false });
  ```
- **`-f ai` filter.** Documented for watching `gen_ai`/`mcp` agent spans
while iterating on an agent. Normalized `sentry local serve -f ai` →
`sentry local -f ai` for consistency.

## Where it lands

- `src/commands/local/run.ts` → `CLIENT_SPOTLIGHT_PREFIXES` constant +
env injection loop; updated module JSDoc and `fullDescription`.
- `test/commands/local/run.test.ts` → asserts every
`<PREFIX>SENTRY_SPOTLIGHT` variant plus the base name are injected with
the spotlight URL.
- `docs/src/fragments/commands/local.md` → the `local` docs page +
generated skill reference.
- `docs/src/content/docs/agent-guidance.md` → the "Capture Events
Locally" workflow. Since the skill generator keeps code blocks but
strips prose, the key facts are encoded as comments in a bash snippet so
they reach the agent skill too.
- Regenerated `SKILL.md`. Unrelated date-based reference drift left out;
CI auto-commit handles that.

## Notes

- Complements #998 (which adds `--verify`/`--timeout`/dev-script
auto-detect to `local run`).
- `check:fragments`, `check:docs-sections`, `lint`, `typecheck`, and the
`test/commands/local` suite (29 tests) pass locally.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Burak Yigit Kaya <byk@sentry.io>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

@betegon betegon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's fix those conflicts and merge this thing!!!

@MathurAditya724

Copy link
Copy Markdown
Member Author

fix-ci: attempt 1 — two lint issues after the merge-from-main:

  1. Formatting in test/commands/local/run.test.ts:283 — biome wanted the func.call() on a single line instead of multi-line (the args fit within the line width)
  2. Unused suppression in src/lib/upgrade.ts:58 — the biome-ignore lint/performance/noBarrelFile comment on a type-only re-export is no longer needed since biome doesn't flag export type for that rule

Both fixed in 5e9f5bf.

@MathurAditya724 MathurAditya724 enabled auto-merge (squash) June 5, 2026 01:15
@MathurAditya724 MathurAditya724 merged commit e0dd2c1 into main Jun 5, 2026
28 checks passed
@MathurAditya724 MathurAditya724 deleted the feat/local-run-verify branch June 5, 2026 01:18
Comment thread src/lib/init/verify-setup.ts
Comment thread src/lib/init/verify-setup.ts
MathurAditya724 added a commit that referenced this pull request Jun 5, 2026
## Summary

SIGINT/SIGTERM during `--verify` mode or post-init verification was
forwarded to the child process but never re-emitted afterward. After
cleanup completed, the CLI continued normally instead of terminating —
the user's Ctrl+C was silently swallowed.

This fix tracks the received signal and re-emits it via
`process.kill(process.pid, signal)` after all cleanup (child kill,
server shutdown, listener removal) completes, so the default handler
terminates the process with the correct exit code.

Fixes both `src/commands/local/run.ts` (`runWithVerify`) and
`src/lib/init/verify-setup.ts` (`verifySetup`).

## Testing

- `vitest run test/commands/local/run.test.ts` — 12 tests pass
- `vitest run test/lib/init/wizard-runner.test.ts
test/lib/dev-script.test.ts test/lib/dev-script.property.test.ts` — 57
tests pass
- `pnpm run lint` — clean
- `tsc --noEmit` — no errors (excluding pre-existing generated-file
issues)

Addresses warden findings from #998.

<!--
## Plan

### Problem
Warden flagged (twice, across commits 5c40365 and fc94462) that SIGINT
is swallowed
in verify-setup.ts: signal handlers forward SIGINT to the child but
don't re-emit or
exit, so the wizard continues after verifySetup returns. The same
pattern exists in
runWithVerify in run.ts.

### Fix (2 files)

1. `src/lib/init/verify-setup.ts`
   - Add `signalReceived: NodeJS.Signals | null` variable
   - Update onSigint/onSigterm to set signalReceived before forwarding
- After cleanup, check signalReceived and re-emit via
process.kill(process.pid, signal)

2. `src/commands/local/run.ts` (runWithVerify function)
   - Same pattern: track signal, re-emit after cleanup

### Verification
- PR #1034's CLIENT_SPOTLIGHT_PREFIXES work confirmed intact on main
- All existing tests pass
- Lint and typecheck clean
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants